Subscribe to Label Action

cc @peterhuene

Thanks for the PR here! I like the look of this and I agree it's probably best to avoid allowing arbitrary implementations of CodeMemory. To make sure I understand what's going on here -- this is assuming that the *.so and *.dll created by your tool maps the *.cwasm into memory but the directives in the native object are such that the memory protections of all attributes are already configured appropriately? For example .text is already executable and everything else is already readonly?

Also, how willing are you to continue to work on this? I realize you're probably focused on what you're working on rather than changing this according to review, but I think some of the points below are going to be important to continue to maintain this over time for us. Additionally I think this feature could be useful to other folks as well, so I think it'd be good to polish it too if we can. That being said I'm happy to help out myself where I can, but I probably can't take on everything below, so your assistance as well would be much appreicated.

If my assumption above is correct, I like this approach! At a high-level though some things I think may want to be changed are:

• Primarily I think we should test this in CI somehow. This sort of feature is ripe for getting broken over time so I think it would be best to have tests. This is, however, probably tricky to do because it would require your cwasm2so tool. That to me points in a slightly different direction, leading me to...
• I think it'd be reasonable to bake in your cwasm2so into Wasmtime itself. We recently added a CodeBuilder structure which is used to configure compilation, and I think this might make a good option for that. Something like builder.create_native_so(true) or something like that so the bytes that pop out of compile_module_serialized look like the ones from cwasm2so you've written. This would, for example, set a flag in CodeBuilder where it does the normal compile process and then more-or-less runs your tool. That's what would enable running tests on CI as well (as we could perhaps literally dlopen)
• For a more bikesheddy point, would it work for your use case to perhaps let Wasmtime dlopen the file? That way we could change the constructor to take a &Path and we'd dlopen it internally. That would help clean up the API a bit for example around the arbitrary destructor.

Yes, its contents are mapped directly so that calling serialize() would yield cwasm original file, with separate segments for executable areas: (screenshots for ELF and PE, with different input files)


Here first three segments contain cwasm file, while second one corresponds to .text section inside it. Remaining segments contain metadata needed for dynamic library, headers pointing to unwind information, etc.

I have taken the view here that generating these binaries and how exactly they are loaded are up to embedder. One thing to note that while I don't need that for my use case, replacing raw memory range with library to be loaded would preclude use of linking compiled module as static library. Nevertheless if that's desired I could work on moving generation and loading into wasmtime itself some time later.

Ok makes sense, thanks for the clarification!

Personally I think it's important to have tests for this, and to do that I think it's ok to move the bits and pieces necessary to build this image into Wasmtime itself. If the bits and pieces in Wasmtime don't work for your use case though then I definitely don't want to ask you to build something you're not going to use.

One of the main worries I have is that there's a lot of implicit assumptions about the output of Wasmtime for this tool to work, so I'm a bit afraid of putting that on embedders as it seems like we may accidentally break it in the future. For example:

• This assumes the output is an ELF, but one day we may switch to platform-specific outputs.
• This assumes the output is a single ELF, but with components we may want to produce multiple outputs one day.
• This assumes various specifics about the number and placement of sections.
• This assumes that there's no relocations to be applied, which is mostly true but won't always be the case.

More-or-less I'd be more comfortable if we internalized some of these pieces in Wasmtime to be able to update it as the design in Wasmtime itself evolves over time. For example if the goal is to create a linkable object I think that'd be great to add here as well. If creating a dynamic object is all that's needed I think your gist would work well to live in Wasmtime too.